-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changed CLI View Prefix #774
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! However, this is an API breaking change and should target next
instead of master
so that it goes into version 2.0. Can you rebase this on next
and change the target branch? (I can help if you want -- it is a quick process but I want to let you try it if haven't before.)
… duplicate view in test. Still need to add tests
e92cb75
to
a58c481
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Before merging, please update the changelog (see the PR checklist for more info). Also the documentation should be reviewed for anything that should be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
* Changed prefix to --prefix to make the argument optional. Changed the duplicate view in test. Still need to add tests * Added shorthand notation to view parser * Added skeleton for test_view_prefix * Extended test_view_incomplete_path_spec to test -p and --prefix * Fixes * Added test for optional prefix view * Added name to contributors * Update tests/test_shell.py Co-authored-by: Bradley Dice <[email protected]>
* Changed prefix to --prefix to make the argument optional. Changed the duplicate view in test. Still need to add tests * Added shorthand notation to view parser * Added skeleton for test_view_prefix * Extended test_view_incomplete_path_spec to test -p and --prefix * Fixes * Added test for optional prefix view * Added name to contributors * Update tests/test_shell.py Co-authored-by: Bradley Dice <[email protected]>
* Changed prefix to --prefix to make the argument optional. Changed the duplicate view in test. Still need to add tests * Added shorthand notation to view parser * Added skeleton for test_view_prefix * Extended test_view_incomplete_path_spec to test -p and --prefix * Fixes * Added test for optional prefix view * Added name to contributors * Update tests/test_shell.py Co-authored-by: Bradley Dice <[email protected]>
* Changed prefix to --prefix to make the argument optional. Changed the duplicate view in test. Still need to add tests * Added shorthand notation to view parser * Added skeleton for test_view_prefix * Extended test_view_incomplete_path_spec to test -p and --prefix * Fixes * Added test for optional prefix view * Added name to contributors * Update tests/test_shell.py Co-authored-by: Bradley Dice <[email protected]>
Description
I changed the CLI
view
argument to establish a path to be a flag rather than a positional argument. Added a test to check this change and altered another test that hadsignac view view path
and changed it to justsignac view path
.Motivation and Context
Fixes issue #653.
Checklist: